add tags #236
add tags #236
Conversation
There was a problem hiding this comment.
Hi @rgl,
Thanks for the PR, looking at the code, the current implementation directly passes through the data to the Proxmox API, so it may make sense to keep it this way for transparency, although I would think an array of strings here to be more idiomatic to Packer, and maybe more resilient to future changes (although I would be surprised if this ever changes).
I'll let you decide if we keep the current implementation or if we move to a string array based implementation with a stirings.Join before pushing it to the API, my preference goes for the latter, but the former may make more sense for people who have qemu experience.
Alternatively may I ask what it looks like on PVE? I don't have an instance to access, but their way to specify the tags should probably be what we adopt for this feature.
builder/proxmox/common/config.go
Outdated
| @@ -93,6 +93,9 @@ type Config struct { | |||
| // If not given, the next free ID on the cluster will be used. | |||
| VMID int `mapstructure:"vm_id"` | |||
|
|
|||
| // The tags to set. | |||
There was a problem hiding this comment.
I would suggest, if we keep the current format with values separated by ;, that we disclose it here so it's clearly described in the docs.
|
I've ended up using the semicolon separated list. At the Proxmox VE Management Console, this is how it looks (notice the tags above the Debian logo):
At the VM definition file, this is how it looks (notice the line that starts with root@pve:~# cat /etc/pve/qemu-server/105.conf
#Packer ephemeral build VM
agent: 1
bios: ovmf
boot: order=scsi0;ide2;net0
cores: 2
cpu: host
efidisk0: local-lvm:vm-105-disk-0,efitype=4m,pre-enrolled-keys=0,size=4M
ide2: local:iso/debian-12.2.0-amd64-netinst.iso,media=cdrom,size=628M
kvm: 1
machine: q35
memory: 2048
meta: creation-qemu=8.0.2,ctime=1696964183
name: packer-65259e1b-00ca-3a70-468a-6c52197d9ba9
net0: virtio=AE:BF:35:97:95:D4,bridge=vmbr0
numa: 0
onboot: 0
ostype: l26
scsi0: local-lvm:vm-105-disk-1,discard=on,iothread=1,size=8G,ssd=1
scsihw: virtio-scsi-single
smbios1: uuid=d10b8b20-da59-4d94-8ac0-0e5b16a32773
sockets: 1
tags: debian-12;template
vga: memory=16,type=qxl
vmgenid: 52bc616a-ae72-43f8-afec-353f060881b0 |
There was a problem hiding this comment.
Hi @rgl,
Thanks for pushing this! The code looks good to me, I'll merge this now
This adds support for tags.
To set the tags, set the
tagsproperty value to a semicolon separated string. For example:I'm still unsure whether to support an array instead, i.e.,
"debian-${var.version};template"vs["debian-${var.version", "template"]. Please advice.Closes #193